Fixes: #41453Closes: #41461
Previously when a model had strict loading set to true and then had a
relation set strict_loading to false the false wasn't considered when
deciding whether to raise/warn about strict loading.
Code example:
```ruby
class Dog < ActiveRecord::Base
self.strict_loading_by_default = true
has_many :treats, strict_loading: false
end
```
In the example, `dog.treats` would still raise even though
`strict_loading` was set to false. This is a bug effecting more than
Active Storage which is why I made this PR superceeding #41461. We need
to fix this for all applications since the behavior is a little
surprising. I took the test from ##41461 and the code suggestion from #41453
with some additions.
Co-authored-by: Radamés Roriz <radamesroriz@gmail.com>
https://github.com/rails/rails/issues/41653 noted an issue where if there's service configured (`config.active_storage.service` is commented out), Blob creation via direct upload crashes:
```
Started POST "/rails/active_storage/direct_uploads" for ::1 at 2021-03-09 13:02:57 -0800
Processing by ActiveStorage::DirectUploadsController#create as JSON
Parameters: {"blob"=>{"filename"=>"banana.jpg", "content_type"=>"image/jpeg", "byte_size"=>577085, "checksum"=>"W/vo/JqBNmJHMCaL+PRlBQ=="}, "direct_upload"=>{"blob"=>{"filename"=>"banana.jpg", "content_type"=>"image/jpeg", "byte_size"=>577085, "checksum"=>"W/vo/JqBNmJHMCaL+PRlBQ=="}}}
Completed 500 Internal Server Error in 12ms (ActiveRecord: 3.3ms | Allocations: 5864)
NoMethodError (undefined method `name' for nil:NilClass):
activestorage (6.1.3) app/models/active_storage/blob.rb:52:in `block in <class:Blob>'
activesupport (6.1.3) lib/active_support/callbacks.rb:427:in `instance_exec'
```
This PR fixes that crash. Blob creation will still fail, but with a more informative error about a `service_name` being required.
If a preview cannot be generated, the IO stream that is captured is empty, resulting in a 0-byte preview file being generated and stored in the Active Storage service.
We came across this because Poppler was failing to generate previews of some PDFs, resulting in 0-byte files. Resizing those "previews" then resulted in a MiniMagick error. The MiniMagick error feels like the right end result if it's attempted on a 0-byte file, what doesn't feel right is `Previewer` proceeding normally if the child process that attempted to capture a preview exited unsuccessfully.
Now, if the previewer child process exits with a non-0 status code, we raise an exception.
In a test environment, rely on the loading of
`:active_support_test_case`.
Introduce the `:active_record_fixture_set` hook for the Active Storage
engine to listen for during the load process in a development
environment (like when running `db:fixtures:load`).
Since this commit moves the task-aware path resolution out of the block
that provided local variables, it recreates part of the fixture
directory path resolution logic.
Fixes a flaky Active Storage test introduced by [rails/rails#41065][],
and improves the documentation.
It seems that the test is covering the backwards compatibility of an
older interface for retrieving records through
`ActiveStorage::Record#find_signed!`. The test itself would pass
unpredictably. To isolate the failure and reproduce it consistently, a
see value was found after some trial and error:
```
SEED=59729 bin/test test/fixture_set_test.rb test/models/attachment_test.rb
```
This _used_ to pass consistently because [rails/rails][#41065]
introduced a call to `fixtures :all`, which introduces more variation in
the database's ID generation sequence. Without that line, `id` values
start at `1`, so the fact that calls to
`ActiveStorage::Attached::One#id` and `ActiveStorage::Blob#id` **both
return `1`** is purely coincidence.
The proposed resolution changes the test slightly. Prior to this change,
the identifier used during retrieval and verification fetched from
`@user.avatar.id`, where `@user.avatar` is an instance of
`ActiveStorage::Attached::One`. The verifier/retriever combination in
that test expected a signed identifier for an `ActiveStorage::Blob`
instance. The change involved retrieving an instance through
`@user.avatar.blob`.
To better emphasize how global the `fixtures :all` declaration is, move
it from the `test/fixture_set_test.rb` file to the `test/test_helper.rb`
file.
[rails/rails#41065]: https://github.com/rails/rails/pull/41065
* Improve Fixture support for Active Storage
Inspired by [76b33aa][], this commit extends the Active Storage
documentation to elaborate on how to declare fixtures.
In support of that, also introduce the `ActiveStorage::FixtureSet.blob`
method for injecting in-line [ActiveStorage::Blob][] attributes directly
into fixture YAML.
[76b33aa]: 76b33aa3d1
[ActiveStorage::Blob]: https://edgeapi.rubyonrails.org/classes/ActiveStorage/Blob.html
* Extra CR for style
* Two-space indention
* Explaining variable didn't explain, inline for style
Co-authored-by: David Heinemeier Hansson <david@loudthinking.com>
This commit allows passing a custom `purpose:` keyword argument to both
`find_signed!` and `signed_id`. This matches the signature of the
`super` methods in ActiveRecord, and it is already possible with the
non-bang [`find_signed`][find_signed] method, since that one doesn't
have an override in ActiveStorage (although perhaps it should, to set
the default :blob_id purpose)
This is useful in cases where you want to further isolate different
types of blobs, for example if they have different levels of
authorization.
[find_signed]: be11d1b6e8/activerecord/lib/active_record/signed_id.rb (L42-L48)
Generating a video preview by capturing only the first frame of a video
is problematic for videos that begin with a fade in from black. By
using keyframe and scene detection that is built in to FFmpeg, we can
generate a more representative preview.
`Minitest.plugin_rails_init` sets `Minitest.backtrace_filter` to
`Rails.backtrace_cleaner` right before tests are run, overwriting the
value set in test_helper.rb.
`Rails.backtrace_cleaner` silences backtrace lines that do not start
with `Rails.root` followed by e.g. "lib/" or "test/". Thus when
`Rails.root` is a subdirectory of the project directory -- for example,
when testing a plugin that has a dummy app -- all lines of the backtrace
are silenced.
This commit adds a fallback such that when all backtrace lines are
silenced, the original `Minitest.backtrace_filter` is used instead.
Additionally, this commit refactors and expands existing test coverage.
The `linguist-vendored` attribute excludes the specified file from the
project's language stats on GitHub. The `linguist-generated` attribute
does the same, and also suppresses that file in diffs on GitHub.
See https://github.com/github/linguist for more information.
Permit applications to hack in custom DB config for ASt models until ASt has first-class multi-DB support:
ActiveSupport.on_load(:active_storage_record) do
connects_to reading: :active_storage_replica, writing: :active_storage_primary
end
rebase
When a PDF is used for both printing and displaying. It will most likely
contain a crop box in order to hide print margins when displaying the PDF.
Use Poppler's parameter to automatically use the crop box (visible box)
rather than the media box (printable box) in order to remove those margins
when drawing the PDF.
See https://manpages.debian.org/testing/poppler-utils/pdftoppm.1.en.html
Guard against the case where ActiveStorage.verifier isn't yet initialized at load time.
Yes, you're not supposed to load AR models in initializers, but it's easy to do accidentally as long as we don't prevent it. We should be resilient against it wherever practical.