REST API: Add tests for the sideload convert_format boolean arg - #12002
REST API: Add tests for the sideload convert_format boolean arg#12002adamsilverstein wants to merge 11 commits into
convert_format boolean arg#12002Conversation
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.
|
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. |
|
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. |
Moving sideload metadata writing to the finalize endpoint adds a sub_sizes argument to that endpoint's schema. Regenerate tests/qunit/fixtures/wp-api-generated.js so the "fixtures are up to date" CI guard (git diff --exit-code) passes.
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. |
Backports the convert_format test coverage from Gutenberg PR #77565. The convert_format argument and its handling already exist in core; this adds the missing test coverage, folding it into this sideload-finalize PR because the second test asserts the sideload response shape introduced here (a top-level file key on the lightweight sub-size response). Supersedes the separate backport PR for GB #77565, whose unique contribution was only these two tests.
|
Added the |
Convert stacked // line comments into /* ... */ block comments for the multi-line explanatory comments added in this change.
@westonruter - that other pr was closed in favor of this one, apologies for any confusion. |
Resolve conflicts in the sideload endpoint after the finalize refactor (WordPress#12002) changed sideload_item to return sub-size data instead of writing attachment metadata inline. Keep the dimension-validation feature: read and reject unreadable images up front, and validate scalar image sizes. Guard the validate_image_dimensions() call behind a scalar check since trunk now allows an array image_size (multiple registered sizes sharing one file), which the string-typed validator cannot accept; array sizes are handled by the per-size branch below.
…oad-finalize # Conflicts: # src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php # tests/phpunit/tests/rest-api/rest-attachments-controller.php # tests/qunit/fixtures/wp-api-generated.js
|
Only tests remain in this PR. Once I confirm they pass against trunk I'll go ahead and commit. |
adamsilverstein
changed the title
convert_format boolean arg
…panion test The convert_format alt-ext-suffix test sideloaded a 100x100 PNG companion under image_size 'original' against a 640x480 JPEG parent. The sideload dimension validation added for the client-side-media route now rejects an 'original' whose dimensions do not match the attachment's recorded dimensions, so the request returned 400 instead of 200 and the test failed across the whole matrix. Sideload the companion as the source-format original size instead. A source-format companion kept beside its JPEG derivative is exactly what that size represents, and it is exempt from dimension validation, so the 100x100 PNG stand-in is accepted. The convert_format suppression the test exercises runs before the size branching, so the test's intent is unchanged.
These tests ride on the sideload-metadata-to-finalize backport PR, which is tracked under #65329 (the ticket that owns this PR). Align their @ticket annotations with that ticket, matching the finalize tests already filed under #65329.
2 participants
What
Adds the
convert_formatboolean-arg test coverage from Gutenberg #77565 — Media uploading: declareconvert_formatas boolean arg on sideload route.Part of the post-restore client-side-media backport stack (see #11324).
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 ([61703]), 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).Trac ticket: https://core.trac.wordpress.org/ticket/65329
Proposed commit message