Backport 'Improve ActiveStorage asset linking performance' to v0.28 #13228
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Improve ActiveStorage asset linking performance Use direct links to the assets living at storage services in order to push most of the application load to the storage service instead of the website itself. * Fix issues serving image variants directly through the storage service The image variants need to be processed through the local representation URLs when they are served the first time because this is when these assets are processed for the first time. These changes detect if the variant needs processing and serves it through the local representation URL before the image has been processed. If the image is already processed it is served directly through the storage service itself. * Fix and extend the storage asset router specs - Fix the storage asset specs to match the current implementation - Add more tests to test the router under different configurations and options - Add an RSpec support utility to define the storage host when the storage URLs are tested outside of a request context * Fix typo * Define the current host for the assets for all specs In order to avoid adding the asset support hooks to too many specs. * Fix spelling issues * Improve the storage router logic for local disk storage - Add more logic for the disk service to determine the availability of the current host before generating the URL - Define the current host based on the record if the record has details about its organization, only if the current host hasn't been set already * Add the ActiveStorage::SetCurrent concern to devise controllers * Fix specs broken due to the changes in the asset URL generation As the URLs are now changed every time they are generated, this caused some specs to fail that assumed the URL of the asset is matches always the format in which it was first generated. The asset URL includes the expiry information and the expiry information is different every time the URL is requested. * Fix spelling in variable name * Different strategy to escape the string included in the regexp * Fix id documents verification flaky spec * Fix further specs expecting blob URLs As the URLs can be different every time they are requested, use different strategy for testing. * Use different matching strategy for questionnaire file answers In order to fix the CodeQL violations. * Fix flaky specs for inviting process users * Fix assembly serializer spec for checking the file URLs * Consider that the asset can be in the generators spec * Fix participatory process group type spec for checking the file URLs * Test the participatory process presenter without setting the current host * Ensure that also exists at the storage service In case the variant is already created at the database but not yet uploaded to the storage service, it might not be available yet. * Method naming and add documentation to the new method - Use the same conventions for the method name as Ruby and Rails - Add method documentation * Add a note about checking that the variant exists at the storage * Add a spec for unexisting and processed variant with record Test the situation when ActiveStorage reports the asset to be processed but it does not exist at the storage service as this caused some of the system specs to fail before. * Use the matcher instead of * Improve the regexp matching in the asset router * Fix passing incompatible URL options to * Simplify logic The options do not need to be merged twice. * Apply suggestions from code review Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro> * Fix unescaped dots in the spec regexps * Fix unescaped host name dots in the spec regexps * Fix rubocop violation * Fix error when trying to display image with empty source The helper requires the image source to be present. Otherwise an error will be thrown. * Use instead of escaping dots only * Fix endless loop in the * Fix ActiveStorage deprecation warnings * Fix translated organization name Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro> * Ensure DiskService availability with current host requirement Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro> * Remove reference to participatory process banner image * Fix broken tests * Fix more specs --------- Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro>
andreslucena
added
backport
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
alecslupu
requested changes
Jul 30, 2024
andreslucena
force-pushed
the
backport/0.28/improve-activestorage-asset-li-12576
branch
from
c655472
to
f452855
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎩 What? Why?
Backport #12576 to v0.28