Regression: wp_update_attachment_metadata filter now fires very often and without complete metadata

[ianmjones]

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.

[azaozz]

Moving to 5.3 for consideration.

At first look:

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 ) );

Right, but wp_update_attachment_metadata() is also used in quite a few other cases. For example when editing an attachment post, in wp_ajax_crop_image(), to update the ID3 data for audio files in wp_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-processing wp_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 to wp_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? :)

[azaozz]

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()?

[ianmjones]

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, in wp_ajax_crop_image(), to update the ID3 data for audio files in wp_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.

[azaozz]

Went through several possible options on how to fix this. It's true that using wp_update_attachment_metadata as a hook that means "file uploaded successfully" is somewhat non-standard. It is intended for filtering the actual data before saving it in the DB.

However it seems several plugins use it to continue post-processing after a file was uploaded. A better hook for that perhaps would be wp_generate_attachment_metadata. At the same time it's true there is a "change in behaviour" for the wp_update_attachment_metadata filter. As the attachment metadata is updated after each image sub-size is generated, now it runs several times in a row.

Seems the best way to fix that new behaviour would be to not run wp_update_attachment_metadata() for interim updates of the meta, only run it at the end after all post-processing has been done successfully. This change will also prevent firing the wp_update_attachment_metadata filter in cases where the post-processing of an image fails after all retry attempts and the upload is deemed "unsuccessful" and the attachment is deleted.

In 48451.diff:

• Do not use wp_update_attachment_metadata() for interim updates of the data while an image is being post-processed after upload.
• Introduce wp_interim_update_attachment_metadata filter for these interim updates.