REST API: Add tests for the sideload convert_format boolean arg (backport GB #77565)#12003
REST API: Add tests for the sideload convert_format boolean arg (backport GB #77565)#12003adamsilverstein wants to merge 8 commits into
Conversation
Backport of Gutenberg PR #75888. Eliminate the read-modify-write race between concurrent sideloads for the same attachment by no longer writing attachment metadata in the sideload endpoint. Instead, sideload returns lightweight sub-size data (dimensions, filename, filesize) which the client accumulates and passes to the finalize endpoint, which writes all collected sub-sizes in a single metadata update. This matches how core generates sub-sizes (one metadata write after all sizes exist) and replaces the earlier per-attachment locking approach that the merged Gutenberg PR ultimately abandoned.
The three sub-size finalize tests added in this backport used the placeholder ticket 62243 (the original client-side media feature ticket) before a dedicated ticket existed. Trac #65329 now tracks this change, so update those @ticket annotations. Pre-existing finalize tests keep 62243.
This backport changes the scaled-image sideload behavior (sideload now returns sub-size data and metadata is written at finalize), so add a 65329 ticket reference alongside the existing 64737.
Backport of Gutenberg PR #77036. When several registered image sizes share the same dimensions, the client generates a single physical file and sends its size names as an array, so the file is registered once and referenced under every matching size. The sideload endpoint's `image_size` parameter (and the finalize endpoint's `sub_sizes[].image_size`) now accept a string or an array of strings. Because rest_is_array() treats scalar strings as single-element lists, the enum is enforced via a validate_callback rather than a oneOf schema. sideload_item() and finalize_item() register the file under each name when an array is given.
Backport of Gutenberg PR #77565. The convert_format boolean arg on the sideload route is already present in core (it was included when client-side media processing was reintroduced), so this backport adds the test coverage from the Gutenberg PR: - test_sideload_route_declares_convert_format_boolean asserts the route schema declares convert_format as a boolean defaulting to true. - test_sideload_convert_format_false_suppresses_alt_ext_suffix verifies that passing convert_format as the string "false" (multipart/form-data semantics) coerces to PHP false, suppressing the image_editor_output_format filter so a companion file sharing the attachment basename is not given a numeric suffix.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Trac Ticket MissingThis pull request is missing a link to a Trac ticket. For a contribution to be considered, there must be a corresponding ticket in Trac. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. More information about contributing to WordPress on GitHub can be found in the Core Handbook. |
| if ( 'original' === $image_size ) { | ||
| $metadata['original_image'] = $sub_size['file']; |
There was a problem hiding this comment.
Here's something that PHPStan picked up on which I wasn't sure about, so I didn't fix:
Offset 'file' might not exist on array{image_size: array<string>|string, width?: int<1, max>, height?: int<1, max>, file?: non-empty-string, mime_type?: non-empty-string, filesize?: int<1, max>, original_image?: non-empty-string}.
The REST API endpoint allows file to be undefined. In that case, this would cause an undefined index error. Should this check if it is set?
| if ( 'original' === $image_size ) { | |
| $metadata['original_image'] = $sub_size['file']; | |
| if ( 'original' === $image_size && isset( $sub_size['file'] ) ) { | |
| $metadata['original_image'] = $sub_size['file']; |
2 participants
What
Core backport of Gutenberg #77565 — Media uploading: declare
convert_formatas boolean arg on sideload route.Part of the post-restore client-side-media backport stack (see #11324). Stacks logically on top of GB #75888 (#12002) and GB #77036 (#12001); diff shown against trunk for standalone-mergeability.
Why
The sideload handler reads
$request['convert_format'], and the client sendsconvert_format: falseinadditionalDatawhen uploading a HEIC companion. Withmultipart/form-data, an undeclared arg arrives as the string"false", which is truthy in PHP - soif ( ! $request['convert_format'] )never fires andadd_filter( 'image_editor_output_format', '__return_empty_array', 100 )is skipped. The default HEIC->JPEG output mapping then survives andwp_unique_filename()'s alt-extension collision check bumps the original to-1while the JPEG derivative stays unsuffixed, so the two companion files drift apart on repeat uploads. Declaring the arg as boolean lets REST coerce"false"-> PHPfalse, the filter fires, and the companions keep a shared basename.How
The controller change is already in Core. When client-side media processing was reintroduced (#11324), the sideload route already declared
convert_formatas a boolean (defaulttrue) andsideload_item()already suppressesimage_editor_output_formatwhen it is false. So this backport is tests only - it adds the coverage from GB #77565 totests/phpunit/tests/rest-api/rest-attachments-controller.php:test_sideload_route_declares_convert_format_boolean- asserts the sideload route declaresconvert_formatas a boolean defaulting totrue.test_sideload_convert_format_false_suppresses_alt_ext_suffix- simulates the HEIC companion flow (PNG stand-in + a local PNG->JPEG mapping) and verifies that passingconvert_formatas the string"false"keeps the companion's shared basename with no numeric suffix.Notes
self::$author_id,self::$test_file,$this->enable_client_side_media_processing(), and: void-style test setup. The behavior test usesimage_size => 'original'(a valid enum value) rather than the Gutenberg test's'original-heic', since Core'simage_sizevalidation strictly enforces the registered-size enum.php -land PHPCS (WordPress-Core).