- This is a fix needed to unblock
https://github.com/rails/rails/pull/50284,
because Active Storage relies on a Active Record bug.
The problem is best understood with a small snippet:
```
blob = ActiveStorage::Blob.new
ActiveStorage::Attachment.new(blob: blob)
ActiveStorage::Attachment.new(blob: blob)
# Currently:
p blob.attachments #=> #<ActiveRecord::Associations::CollectionProxy []>
# Once the Active Record bug is fixed:
p blob.attachments #=> #<ActiveRecord::Associations::CollectionProxy [#<ActiveStorage::Attachment id: nil, name: nil, record_type: nil, record_id: nil, blob_id: nil, created_at: nil>, #<ActiveStorage::Attachment id: nil, name: nil, record_type: nil, record_id: nil, blob_id: nil, created_at: nil>]>
# Trying to save the blob would result in trying to create 2 attachments which
# fails because of unique constrainsts.
```
### Code path
The real code path that does what the snippet above does is located here:
9c3ffab47c/activestorage/lib/active_storage/attached/many.rb (L52)
It's basically doing this:
```
user.images.attach "photo1.png"
# Initialize a Blob record and an Attachment
user.images.attach "photo2.png"
# Get the Blob from above, create another Attachment
# Initialize a new Blob record and an new Attachment
# rinse and repeat every time `attach` is called
```
Basically each time we call `attach`, we grab the previous blobs that were attached
(and that already have an Attachment record), and
### Solution
- Explicitly set the `inverse_of`, so that it behaves as if #50284 is shipped
- Don't build a new attachment for blob already having one.
### Tests
I didn't add tests, the test suite is already well covered, adding the `inverse_of`
without any changes breaks the test suite already. You can try by running
for instance the `activestorage/test/models/attached/many_test.rb`.
For non-image attachments (like videos), generated representations are
created as a preview_image_attachment instead of as normal variants, and
as a result aren't included in the `with_attached_` scopes. This adds
those preview image attachments to the `includes` of these scopes to
avoid an N+1 when iterating over a collection of attachments and
fetching the key of their representation (variants).
Co-Authored-By: Justin Searls <searls@gmail.com>
* Remove pidfile in production
* Update changelog
* Update activestorage/test/dummy/config/puma.rb
Co-authored-by: Rafael Mendonça França <rafael@franca.dev>
* Update template and other dummy files
---------
Co-authored-by: Rafael Mendonça França <rafael@franca.dev>
RDoc treats consecutive indented lines as a single code block. For code
examples that span multiple files / languages, this confuses the syntax
highlighter and makes the examples harder to read. Unfortunately, RDoc
doesn't provide syntax to prevent this, and it ignores multiple
consecutive blank lines. However, by inserting an empty tag such as
`<code></code>`, we can force RDoc to recognize separate code blocks.
Now that we dropped support for Ruby 2.7, we no longer
need to check if variables are defined before accessing them
to avoid the undefined variable warning.
Adds space inside array literal brackets in ActiveStorage/ActionText migrations.
The new `rubocop-rails-omakase` enables `Layout/SpaceInsideArrayLiteralBrackets`,
which failed on the migrations created when installing ActiveStorage and ActionText.
Until now, Rails only droped compatibility with older
rubies on new majors, but I propose to change this policy
because it causes us to either keep compatibility with long
EOLed rubies or to bump the Rails major more often, and to
drop multiple Ruby versions at once when we bump the major.
In my opinion it's a bad alignments of incentives. And we'd
be much better to just drop support in new minors whenever they
go EOL (so 3 years).
Also Ruby being an upstream dependency, it's not even
a semver violation AFAICT.
Since Rails 7.2 isn't planned before a few months, we
can already drop Ruby 3.0 as it will be EOL in March.
To assert the expected number of queries are made, Rails internally uses
`assert_queries` and `assert_no_queries`. These assertions can be
useful in applications as well.
By extracting these assertions to a module, the assertions can be
included where required.
These assertions are added to `ActiveSupport::TestCase` when
ActiveRecord is defined.
ActiveStorage, ActionView and ActionText are using this module now as
well, instead of duplicating the implementation.
The internal ActiveRecord::TestCase, used for testing ActiveRecord,
implements these assertions as well. However, these are slighlty more
advanced/complex and use the SQLCounter class. To keep things simple,
for now this implementation isn't used.
Add guidance to the Association Basics and `.belongs_to` method
documentation to encourage the renaming of a model's Ruby class to
coincide with updates to the existing data in the database.
Since Action Text and Active Storage rely on polymorphic associations,
add similar warnings to their guides.
Co-authored-by: Petrik de Heus <petrik@deheus.net>
Co-authored-by: Stephen Hanson <s.hanson5@gmail.com>
Co-authored-by: zzak <zzakscott@gmail.com>
All of ActiveStorage database modeltable nameshave been hard coded.
Therefore, ActiveRecord::Base.(prefix|suffix) were not taken into
consideration. To fix this we remove the hard coded lines. But then we
need to also override an internal method for specifying the prefix
because of a mystical ActiveRecord/ActiveStorage sync issue
(Suffix does not appear to have the issue)
Some tests were refactored to remove hard coded table name references,
making ActiveStorage test suite compatible with ActiveRecord config.
Follow-up to #48290.
The `:is(button, input)[type='submit']` selector does not match `button`
elements that omit the `type` attribute in favor relying on its default
value (which is `"submit"`).
Co-authored-by: Javan Makhmali <javan@javan.us>
When a blob is a representable of kind `previewable`, the preview
image that's being proxied is always the original preview image,
discarding completely the `variation_key` param passed in the request.
This commit fixes this by editing `Preview` and `VariantWithRecord` to
have full synchronized API with `Variant`. this will then allow the
ProxyController to not call `representable#image` but `representable`
instead.
As all 3 classes are now 100% interchangeable, we can deprecate the use
of `Representable#image`. Users of this class won't need to call this
method as it's become obsolete. and in case of `Preview#image`
erroneous.
`ActiveStorage::Variant`, the class used to handle untracked variants,
is lacking some methods to make it compliant with
`ActiveStorage::Representations::ProxyController#send_blob_stream`.
This commit fixes the proxying of untracked variant by adding the
missing methods.
Preprocessing "unpresentable" blobs (neither variabe, nor previewable)
will result in a `ActiveStorage::UnrepresentableError`, which will retry
following `ActiveJob::Base` default retry logic.
This commit discards any blob that's not representable to cleanup the
job adapter queue.
if an empty hash is passed to a preview call (`blob.preview({})`)
We go through the original preview instead of regenerating a variation
based on the original preview image which would result in a performance
penalty
This change is necessary to address a potential issue that could arise when a button or an input of type submit contains child elements, such as spans, icons, or other HTML elements.
Currently, ActiveStorage's `didClick` event listener checks the target of the click event to determine if a submit button was clicked. The target property of the event refers to the specific HTML element that was clicked.
In cases where a submit button contains child elements, and one of these child elements is the element that actually gets clicked, the target would refer to this child element, not the button itself.
Since the `didClick` function checks if the target is a button or an input of type submit, this check would fail, and the button wouldn't be stored in `submitButtonsByForm`.
As a result, if the form is then submitted after a direct upload, the first submit button in the form could be incorrectly used to submit the form, even if a different button was originally clicked. This could cause unexpected behavior, as different submit buttons might be intended to trigger different actions on form submission.
By using the `event.currentTarget` instead, we'll get back the button or an input of type submit. This way we ensure that the correct button is stored in `submitButtonsByForm`, even if the click event was triggered by a child element of the button. This addresses the issue and ensures that the correct button is used to submit the form after a direct upload.
https://developer.mozilla.org/en-US/docs/Web/API/Event/currentTarget
Prior to this commit, `ActiveStorage::Preview#processed` would only
process the preview image, not the specified variant of the preview
image. For example, `thumb = attached_pdf.preview(:thumb).processed`
would only generate the full-sized preview image, not the `:thumb`
variant of it, until e.g. `thumb.url` was called.
This commit updates `ActiveStorage::Preview#processed` to generate both
the full-sized preview image and the requested variant.
Co-authored-by: chaadow <chedli@hoggo.com>
`ActiveStorage::Preview#url` delegates to the preview image's variant,
which in turn delegates to the variant's blob. Thus when the variant
has already been processed and strict loading is enabled, the
association chain of `preview_image_attachment` => `blob` =>
`variant_records` => `image_attachment` => `blob` must be fully
pre-loaded; otherwise, `ActiveStorage::Preview#url` will raise an
`ActiveRecord::StrictLoadingViolationError`.
This ensures `ActiveRecord::Base.strict_loading_by_default` and
`ActiveStorage.track_variants` are reset to their original values even
when an error (e.g. an assertion failure) is raised inside
`with_strict_loading_by_default` and `without_variant_tracking` blocks.
The transform_job crahes if you want to preprocess a previewable files.
This commit fixes that by using the blob's `representation` method to
process variants or previews.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
This adds `linguist-generated` and `linguist-vendored` attributes where
appropriate to suppress the files in diffs and exclude the files from
the project's language stats on GitHub.
See https://github.com/github/linguist for more information.
Prior to this commit, if `app/javascript/activestorage/index.js`
contained a syntax error, `JavascriptPackageTest` would still pass
because `system "yarn build"` would simply return `false` and the
compiled output would not change. This commit adds `exception: true` to
the `system` call so that an error will be raised if `yarn build` fails.
Also, since 6c96e1cd7b, Active Storage
compiles an additional `app/assets/javascripts/activestorage.esm.js`
file. This commit adds an assertion for that file as well.
Remove the option `config.public_file_server.enabled` from the generators for all environments, as the value is the same in all environments.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
`service` kwarg in `has_one_attached` and `has_many_attached` methods accepts
only symbols as values. It allows to define a `service` at a class-level context. But in
one of our requirements, we wanted to upload files based on user's region due to
some regulations.
So in order to allow defining a `service` at instance-level context,
this PR makes the changes to accept `service` as a Proc as well.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
FFmpeg 6.0 now reports the duration of `video_without_video_stream.mp4`
as 1.000000 instead of 1.022000. This discrepancy was [reported][]
to the FFmpeg mailing list, and the [reply][] indicated that the change
is intentional.
For this test, the exact duration isn't significant. We merely want to
assert that the metadata includes the duration reported by FFmpeg.
Therefore, this commit changes the assertion to accomodate the duration
reported by FFmpeg 6.0 as well as previous versions.
Fixes#49650.
[reported]: https://ffmpeg.org/pipermail/ffmpeg-user/2023-October/057067.html
[reply]: http://ffmpeg.org/pipermail/ffmpeg-user/2023-October/057083.html
Co-authored-by: Yasuo Honda <yasuo.honda@gmail.com>
Follow-up to #47137.
Since `config.public_file_server.enabled` is true by default, this
commit changes the `config/environments/production.rb` template to
present the setting as an opt-out.
For better or worse, the Rails guide settled on double quotes
and a large part of the community also use rubocop which enforce
them by default.
So we might as well try to follow that style when providing code
snippets in the documentation or error messages.
Fix: https://github.com/rails/rails/issues/49822
I certainly didn't get them all, but consistency should be significantly
improved.
ActiveStorage::Attachment records are not directly maintained by
ActiveStorage and it may not be feasible or desired to update the record
when its attachment is saved.
Right now we are using both to test the Rails applications we generate
and to test Rails itself. Let's keep CI for the app and BUILDKITE to
the framework.
It's possible since Rails 6 (3ea2857943) to let the framework create Event objects, but the guides and docs weren't updated to lead with this example.
Manually instantiating an Event doesn't record CPU time and allocations, I've seen it more than once that people copy-pasting the example code get confused about these stats returning 0. The tests here show that - just like the apps I've worked on - the old pattern keeps getting copy-pasted.
Edouard CHIN