![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 2 months ago
Closed 2 weeks ago
#65329 closed enhancement (fixed)
Move sideload metadata writes to the finalize endpoint
| Reported by: | adamsilverstein | Owned by: | adamsilverstein |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.1 |
| Component: | Media | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests needs-testing |
| Cc: | Focuses: |
Description
Backport - part of the client-side media processing feature being re-introduced for 7.1 (see #64919, originally #62243).
When client-side media processing is enabled, the editor generates each image sub-size in the browser and sideloads it to the attachment via POST /wp/v2/media/<id>/sideload. Today each sideload request reads _wp_attachment_metadata, merges in its own sub-size, and writes the whole array back. Because the editor uploads sub-sizes for a single attachment concurrently (bounded only by maxConcurrentUploads, default 5), two requests can read the same metadata, each add their own size, and the later write clobbers the earlier one. The result is missing sub-sizes in the saved metadata.
The merged Gutenberg PR explored MySQL advisory locks and transient-based locks before settling on the approach in this ticket: remove the shared write from the per-size request entirely.
Proposed change
sideload_item()no longer writes_wp_attachment_metadata. Instead it returns lightweight sub-size data for the size it just processed:image_size,width,height,file,mime_type,filesize, andoriginal_imagefor the scaled case. The scaled flow still repoints_wp_attached_fileviaupdate_attached_file(), which is per-attachment and not the contended_wp_attachment_metadata.finalize_item()accepts a new schema-validatedsub_sizesarray argument. The client accumulates the sub-size data returned by each sideload response and passes the full set to finalize, which applies them all in a singlewp_update_attachment_metadata()call before firingwp_generate_attachment_metadata.
This mirrors how Core itself builds sub-sizes: every size is created first, then metadata is written once. Existing image_meta (EXIF) on the attachment is preserved when finalize merges in the collected sub-sizes.
Why this is safe
- The contended write (
_wp_attachment_metadata) now happens exactly once, in finalize, instead of once per concurrent sideload. sub_sizesis fully schema-validated inregister_routes(): per-itemimage_sizeis required,width/height/filesizeare positive integers, andmime_typematches^image/.*.- The empty-
sub_sizespath is preserved, so a finalize call with no collected sizes behaves exactly as before.
Patch
The PHP change is a backport of the merged Gutenberg PR:
- Gutenberg PR: WordPress/gutenberg#75888, Upload Media: Enable concurrent sideload uploads
- Core backport PR: adamsilverstein/wordpress-develop#48
Files touched:
src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.phptests/phpunit/tests/rest-api/rest-attachments-controller.php
The JavaScript counterpart (media-utils sideloadToServer() returning sub-size data, and the client-side accumulation passed to finalize) ships through the normal Gutenberg to Core package sync and is not part of this PHP patch.
Tests
test_sideload_scaled_imageupdated to assert the new response shape (sub-size data, not a prepared attachment response) and that the attached file is repointed during sideload while metadata is only written after finalize.- Added
test_finalize_writes_regular_sub_sizes(regular size branch),test_finalize_writes_original_metadata(theoriginalbranch), andtest_finalize_preserves_image_meta(EXIFimage_metasurvives a finalize that adds sub-sizes). - Together with the existing
test_finalize_item*tests, all three finalize branches (regular / scaled / original) plus the empty-sub_sizespath are covered.
Change History (8)
This ticket was mentioned in PR #12002 on WordPress/wordpress-develop by @adamsilverstein.
7 weeks ago
#1
@westonruter commented on PR #12002:
7 weeks ago
#2
This also seems to have much of the same code as https://github.com/WordPress/wordpress-develop/pull/12003, which I've reviewed in depth, so I guess that should be committed first before proceeding here.
#3
@
6 weeks ago
@adamsilverstein would you mind adding testing steps to make the changes easier to review?
#4
@
5 weeks ago
Tested the GitHub pull request linked from this ticket using the WordPress Playground preview.
Uploaded multiple large JPEG images (5659x3772 and 5472x3648) and verified uploads completed successfully.
Confirmed WordPress generated scaled image files, attachment metadata loaded correctly, dimensions and file information were displayed, and original image links remained available.
Inserted uploaded images into content, published a post, and verified images displayed correctly on the front end.
Also verified the Edit Image workflow loads successfully and image editing controls (crop, scale, rotate) remain functional.
Did not observe any regressions during testing.
@adamsilverstein commented on PR #12002:
3 weeks ago
#5
Added the convert_format boolean-arg test coverage here in 1b41141231 (backport of Gutenberg #77565), folding in the former #12003. The argument itself already exists in core; these two tests assert it is declared as a boolean and that convert_format=false suppresses the alt-extension suffix on companion files — which relies on the sub-size response shape this PR introduces.
@adamsilverstein commented on PR #12002:
3 weeks ago
#6
This also seems to have much of the same code as #12003, which I've reviewed in depth, so I guess that should be committed first before proceeding here.
@westonruter - that other pr was closed in favor of this one, apologies for any confusion.
@adamsilverstein commented on PR #12002:
2 weeks ago
#7
Only tests remain in this PR. Once I confirm they pass against trunk I'll go ahead and commit.
## What
Core backport of Gutenberg #75888 — Upload Media: Enable concurrent sideload uploads.
Part of the post-restore client-side-media backport stack (see #11324).
## Why
Concurrent sideloads for the same attachment each did a read-modify-write of
_wp_attachment_metadata, so parallel requests could overwrite each other's changes. The merged Gutenberg PR explored MySQL advisory locks and transient locks, then settled on removing the shared write entirely.## How
sideload_item()no longer writes attachment metadata. It returns lightweight sub-size data (image_size, dimensions,file,mime_type,filesize, andoriginal_imagefor the scaled case). The scaled flow still repoints_wp_attached_file(that is not the contended_wp_attachment_metadata).finalize_item()accepts a newsub_sizesarray param (schema-validated) and applies all collected sub-sizes to the metadata in a singlewp_update_attachment_metadata()before firingwp_generate_attachment_metadata.This matches how core generates sub-sizes (one metadata write after all sizes exist).
## Notes
sideloadToServerreturning sub-size data; client-side accumulation passed to finalize) ships via the normal Gutenberg -> Core package sync and is not part of this PHP backport.test_sideload_scaled_imageto assert the new response shape + finalize write, and added three finalize tests -test_finalize_writes_regular_sub_sizes,test_finalize_writes_original_metadata(covers theoriginalbranch), andtest_finalize_preserves_image_meta(EXIFimage_metais preserved when finalize adds sub-sizes). Together with the existingtest_finalize_item*tests, all three finalize branches (regular / scaled / original) plus the empty-sub_sizespath are covered. Validated locally withphp -land PHPCS (WordPress-Core).