REST API: Add tests for the sideload convert_format boolean arg (backport GB #77565)#50
Closed
adamsilverstein wants to merge 5 commits into
Closed
REST API: Add tests for the sideload convert_format boolean arg (backport GB #77565)#50adamsilverstein wants to merge 5 commits into
adamsilverstein wants to merge 5 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5 tasks
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.
93c823b to
9fd5b2e
Compare
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.
00738dc to
cb8b288
Compare
Owner
Author
|
Closing in favor of upstream PR (re-opening on WordPress/wordpress-develop with same head branch). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Core backport of Gutenberg #77565 — Media uploading: declare
convert_formatas boolean arg on sideload route.Stacked on #49 (GB #77036) → #48 (GB #75888) →
reintroduce-client-side-media(#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 (WordPress#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 (added by GB #77036 / REST API: Support registering one sideloaded file under multiple sizes (backport GB #77036) #49) strictly enforces the registered-size enum.php -land PHPCS (WordPress-Core). The full PHPUnit suite was not run locally (the wordpress-develop test env is not currently provisioned, and the suite does not run in CI on fork PRs to a non-default base branch).Part of the post-restore client-side-media backport stack.