Opened 3 days ago
Last modified 2 days ago
#48451 new defect (bug)
Regression: wp_update_attachment_metadata filter now fires very often and without complete metadata
Reported by: | ianmjones | Owned by: | |
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | trunk |
Component: | Media | Keywords: | needs-patch 2nd-opinion |
Focuses: | Cc: | ||
PR Number: |
Description
WP5.3 has changed how often the wp_update_attachment_metadata
filter is fired when an image is uploaded to the Media Library.
It fires initially with just the full
(and potentially original_image
) details but no sizes
, and then again as each image sub-size is generated and added to the sizes
array.
With wp_update_attachment_metadata
now firing a minimum of 5 times (10 for "large" files) instead of once, I predict quite a resource hit from plugins processing the filter. There's plenty of image related plugins that hang off that filter to trigger optimizations and other media related processes assuming that the upload is now complete and thumbnails available.
We regularly see customers with 25-40 or more registered image sizes, that's going to mean rather a lot of database calls to update the metadata as each sub-size is generated, followed by plugins processing the image(s) each time.
Maybe the solution is for wp_update_attachment_metadata()
to gain an $unfiltered
param that is set to true during initial upload and sub-size generation, with a final call to the function without $unfiltered when done and dusted?
A common pattern throughout WP5.2 (and still in WP5.3) is as follows...
wp_update_attachment_metadata( $id, wp_generate_attachment_metadata( $id, $file ) );
Such as in \media_handle_upload()
and friends.
So that's why a lot of plugins wait for the complete metadata to be generated (wp_generate_attachment_metadata
) and then saved (wp_update_attachment_metadata
) before handling the new upload.
Change History (5)
#1
follow-up:
↓ 3
@
3 days ago
- Keywords needs-patch 2nd-opinion added
- Milestone changed from Awaiting Review to 5.3
#2
@
3 days ago
Looking again at the proposed wp_update_image_subsizes
action in https://core.trac.wordpress.org/attachment/ticket/47872/47872.3.diff.
Think the problem reported here is related and would be better to handle that on this ticket. Both are about firing a hook after an image has been uploaded and all sub-sizes were created successfully.
Perhaps it would be better to fire wp_generate_attachment_metadata
and then save the meta again with wp_update_attachment_metadata()
at the end of wp_update_image_subsizes()
?
#3
in reply to:
↑ 1
@
3 days ago
Replying to azaozz:
Right, but
wp_update_attachment_metadata()
is also used in quite a few other cases. For example when editing an attachment post, inwp_ajax_crop_image()
, to update the ID3 data for audio files inwp_ajax_save_attachment()
, when editing an image (crop, rotate, ...), for custom background and header images, etc.
Seems using the same filter to detect when images have been uploaded successfully is problematic?
I should clarify the problem here, it's not so much that the wp_update_attachment_metadata
is used for knowing when an upload is complete, it is more used to know when the image and its sub-sizes have potentially changed.
So initially yes, on a new upload we (WP Offload Media) use it as a signal that an image has been uploaded and all of its thumbnails are present and correct and ready to be sent on their merry way to a storage provider.
However, if thumbnails are regenerated, or an image optimizer plugin batch job processes the image and updates the thumbnails, they too generally call wp_update_attachment_metadata
when they are done with the Media Library item. And again, that means plugins like WP Offload Media can re-offload the images to S3/GCS/DO Spaces etc.
The problem with the new setup is that it fires wp_update_attachment_metadata
a lot, and with partial metadata (e.g. no sizes array initially). Which is very different to how it has been.
Moving to 5.3 for consideration.
At first look:
Right, but
wp_update_attachment_metadata()
is also used in quite a few other cases. For example when editing an attachment post, inwp_ajax_crop_image()
, to update the ID3 data for audio files inwp_ajax_save_attachment()
, when editing an image (crop, rotate, ...), for custom background and header images, etc.Seems using the same filter to detect when images have been uploaded successfully is problematic?
Until now the
wp_generate_attachment_metadata
filter was the (de facto) "upload is successful" hook. This is still the case in n 5.3, however when the initial upload request fails and the client makes another request to finish image post-processingwp_generate_attachment_metadata
is not fired. There is a proposed "action" in (the new)wp_update_image_subsizes()
, see https://core.trac.wordpress.org/attachment/ticket/47872/47872.3.diff.Thinking that adding an
$unfiltered
param towp_update_attachment_metadata()
and avoid filtering would be wrong. Other plugins may be using that filter for other purposes, for example moving/updating/fixing the paths to the image sub-sizes.It's also possible that the image attachment meta data is stored temporarily "somewhere" while image sub-sizes are being created (another post meta?), then "moved" to the proper place. At first look not liking much that workaround.
Any other ideas and/or possible fixes? :)