Skip to content

Force resized images to be regenerated when the source image changes#178

Merged
LukeTowers merged 1 commit intowip/1.2from
fix/resize-on-replacement
May 12, 2022
Merged

Force resized images to be regenerated when the source image changes#178
LukeTowers merged 1 commit intowip/1.2from
fix/resize-on-replacement

Conversation

@LukeTowers
Copy link
Member

This should fix #177, but it will cause all existing installations to resize all of their already resized images.

Another alternative could have been to include the hash of the source image; but it would have been at the expense of more work every time this function is called, which could potentially include generating hashes of multiple extremely large (10+ MB) images on every page load if the page includes such images that need to be resized since the filter needs to identify the configuration in order to identify the path to the resized image to check if it needs to return a resizer/ URL or a /resized one.

Further improvements could potentially be made in the isResized() / getPathToResized() methods to cache the results to avoid doing all the work involved with the getConfig() method while still allowing people to run a command to trigger a refresh.

This should fix #177, but it will cause all existing installations to resize all of their already resized images. 

Another alternative could have been to include the hash of the source image; but it would have been at the expense of more work every time this function is called, which could potentially include generating hashes of multiple extremely large (10+ MB) images on every page load if the page includes such images that need to be resized since the filter needs to identify the configuration in order to identify the path to the resized image to check if it needs to return a resizer/ URL or a /resized one.

Further improvements could potentially be made in the isResized()
@LukeTowers LukeTowers added maintenance PRs that fix bugs, are translation changes or make only minor changes Status: Testing Needed needs review Issues/PRs that require a review from a maintainer labels May 15, 2021
@bennothommo bennothommo removed the needs review Issues/PRs that require a review from a maintainer label May 16, 2021
@LukeTowers
Copy link
Member Author

@bennothommo did you have any ideas for a better experience for this:

it will cause all existing installations to resize all of their already resized images.

@bennothommo
Copy link
Member

@LukeTowers I personally don't see a problem with that. The resized images should be considered temporary and treated as a "cache" so to speak.

@LukeTowers
Copy link
Member Author

@bennothommo fair enough, but if we treat it that way then we need to provide a way to clear the cache. Will wait to merge this until we can add a util command to purge the resized images cache.

@LukeTowers
Copy link
Member Author

With that as well, should we provide a migration that clears that cache as a part of the update to avoid unnecessarily cluttering up people's storage directory with now-invalid images or is the added server load of regenerating all existing images not worth it (especially if they're caching the generated HTML which could include references to final files that would no longer exist).

@bennothommo
Copy link
Member

I would envisage that the server load would be staggered, as the resizing only happens on an "as-needed" basis, when people load up a particular page with the resized image(s), so I don't think that's too much of a concern.

We definitely will need some form of command though to clear the cached resized images, if cache:clear doesn't already do that.

I'm not sure what we can do about people caching the generated HTML (and ostensibly, the resized image's direct URL). I would hope that doing a cache:clear as part of this proposed migration to clear the resized images would also reset the page cache as well. The same problem would've occurred with the previous resize functionality as well.

@helmutkaufmann
Copy link
Contributor

@bennothommo fair enough, but if we treat it that way then we need to provide a way to clear the cache. Will wait to merge this until we can add a util command to purge the resized images cache.

I think it would not be about clearing the actual Cache, it would also be about removing all the resized images. But perhaps that is what you meant anyway.

@bennothommo bennothommo added this to the v1.1.5 milestone Jun 17, 2021
@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@github-actions github-actions bot added the stale Issues/PRs that have had no activity and may be archived label Aug 17, 2021
@LukeTowers LukeTowers removed the stale Issues/PRs that have had no activity and may be archived label Aug 17, 2021
@LukeTowers LukeTowers modified the milestones: v1.1.7, v1.2.0 Sep 23, 2021
@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@github-actions github-actions bot added the stale Issues/PRs that have had no activity and may be archived label Nov 23, 2021
@bennothommo bennothommo removed the stale Issues/PRs that have had no activity and may be archived label Nov 23, 2021
@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@github-actions github-actions bot added the stale Issues/PRs that have had no activity and may be archived label Jan 23, 2022
@LukeTowers LukeTowers removed the stale Issues/PRs that have had no activity and may be archived label Jan 23, 2022
@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@github-actions github-actions bot added the stale Issues/PRs that have had no activity and may be archived label Mar 25, 2022
@LukeTowers LukeTowers removed the stale Issues/PRs that have had no activity and may be archived label Mar 25, 2022
@LukeTowers LukeTowers changed the base branch from develop to wip/1.2 May 12, 2022 21:12
@LukeTowers LukeTowers merged commit 78a7201 into wip/1.2 May 12, 2022
@LukeTowers LukeTowers deleted the fix/resize-on-replacement branch May 12, 2022 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance PRs that fix bugs, are translation changes or make only minor changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

resize - Cached resized image provided even if it the original changed

3 participants