Skip to content

Commit 046f381

Browse files
Media: Prepare the attachment finalize response from the current post data.
`finalize_item()` prepares its response from the `$post` fetched before the `wp_generate_attachment_metadata` filter runs, so anything a callback writes to the post row is missing from the response. The editor now stores that response as the attachment record instead of refetching, so re-read the post after the metadata is written and return the `WP_Error` if a callback deleted the attachment. See WordPress/gutenberg#81844 and WordPress/gutenberg#81947. Reviewed by joedolson. Merges [63564] to the 7.1 branch. Props gregbenz, andrewserong. Fixes #66056.i git-svn-id: https://develop.svn.wordpress.org/branches/7.1@63572 602fd350-edb4-49c9-b593-d223f7449a82
1 parent fb84620 commit 046f381

2 files changed

Lines changed: 213 additions & 0 deletions

File tree

src/wp-includes/rest-api/endpoints/class-wp-rest-attachments-controller.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3453,6 +3453,21 @@ public function finalize_item( WP_REST_Request $request ) {
34533453
$response_request['_fields'] = $request['_fields'];
34543454
}
34553455

3456+
/*
3457+
* Re-read the post. The 'wp_generate_attachment_metadata' filter above
3458+
* runs long after $post was fetched, and a callback that rewrites the
3459+
* post row - an optimizer changing post_mime_type once it has
3460+
* converted the file, say - would otherwise be missing from this
3461+
* response. The editor stores the response as its copy of the record
3462+
* rather than reading the attachment again, so a stale row here is
3463+
* what it keeps. A callback that deleted the attachment instead leaves
3464+
* nothing to respond with, so that is reported as the error it is.
3465+
*/
3466+
$post = $this->get_post( $attachment_id );
3467+
if ( is_wp_error( $post ) ) {
3468+
return $post;
3469+
}
3470+
34563471
return $this->prepare_item_for_response( $post, $response_request );
34573472
}
34583473
}

tests/phpunit/tests/rest-api/rest-attachments-controller.php

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5128,6 +5128,204 @@ public function test_finalize_preserves_image_meta(): void {
51285128
$this->assertSame( $original_image_meta['iso'], $metadata['image_meta']['iso'], 'ISO should be preserved.' );
51295129
}
51305130

5131+
/**
5132+
* Verifies that the finalize response carries the generated sub-sizes.
5133+
*
5134+
* The response is prepared after the sub-size metadata has been written, so
5135+
* it is the finished attachment record. The editor stores it as-is instead
5136+
* of fetching the attachment again to pick the sizes up.
5137+
*
5138+
* @ticket 66056
5139+
*
5140+
* @covers WP_REST_Attachments_Controller::finalize_item
5141+
*/
5142+
public function test_finalize_response_contains_generated_sub_sizes(): void {
5143+
$this->enable_client_side_media_processing();
5144+
5145+
wp_set_current_user( self::$author_id );
5146+
5147+
$request = new WP_REST_Request( 'POST', '/wp/v2/media' );
5148+
$request->set_header( 'Content-Type', 'image/jpeg' );
5149+
$request->set_header( 'Content-Disposition', 'attachment; filename=finalize-response-test.jpg' );
5150+
$request->set_param( 'generate_sub_sizes', false );
5151+
$request->set_body( (string) file_get_contents( DIR_TESTDATA . '/images/canola.jpg' ) );
5152+
5153+
$response = rest_get_server()->dispatch( $request );
5154+
$data = $response->get_data();
5155+
$attachment_id = $data['id'];
5156+
5157+
// Nothing has generated sub-sizes yet, which is the window in which the
5158+
// editor's first read of the attachment happens.
5159+
$this->assertEmpty(
5160+
(array) $data['media_details']['sizes'],
5161+
'The create response should not carry sub-sizes yet.'
5162+
);
5163+
5164+
$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" );
5165+
$request->set_header( 'Content-Type', 'image/jpeg' );
5166+
$request->set_header( 'Content-Disposition', 'attachment; filename=finalize-response-test-150x150.jpg' );
5167+
$request->set_param( 'image_size', 'thumbnail' );
5168+
$request->set_body( (string) file_get_contents( DIR_TESTDATA . '/images/test-image.jpg' ) );
5169+
5170+
$response = rest_get_server()->dispatch( $request );
5171+
$thumbnail_data = $response->get_data();
5172+
$this->assertSame( 200, $response->get_status(), 'Sideloading the thumbnail should succeed.' );
5173+
5174+
$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" );
5175+
$request->set_param( 'sub_sizes', array( $thumbnail_data ) );
5176+
5177+
$response = rest_get_server()->dispatch( $request );
5178+
$this->assertSame( 200, $response->get_status() );
5179+
5180+
$data = $response->get_data();
5181+
$this->assertArrayHasKey( 'media_details', $data );
5182+
$this->assertArrayHasKey(
5183+
'thumbnail',
5184+
$data['media_details']['sizes'],
5185+
'The finalize response should list the sideloaded sub-size.'
5186+
);
5187+
5188+
// source_url is what the Image block's Resolution control offers.
5189+
$this->assertArrayHasKey(
5190+
'source_url',
5191+
$data['media_details']['sizes']['thumbnail'],
5192+
'Each sub-size in the finalize response should carry its URL.'
5193+
);
5194+
$this->assertStringEndsWith(
5195+
'finalize-response-test-150x150.jpg',
5196+
$data['media_details']['sizes']['thumbnail']['source_url']
5197+
);
5198+
5199+
// The finalize response must match what a later read would return, as
5200+
// the editor stores it in place of that read.
5201+
$request = new WP_REST_Request( 'GET', "/wp/v2/media/{$attachment_id}" );
5202+
$request->set_param( 'context', 'view' );
5203+
$fetched = rest_get_server()->dispatch( $request )->get_data();
5204+
5205+
$this->assertSame(
5206+
array_keys( (array) $fetched['media_details']['sizes'] ),
5207+
array_keys( (array) $data['media_details']['sizes'] ),
5208+
'The finalize response should carry the same sizes a refetch would.'
5209+
);
5210+
}
5211+
5212+
/**
5213+
* Verifies that the finalize response reflects the post row as it stands
5214+
* after the metadata has been generated.
5215+
*
5216+
* finalize_item() applies the 'wp_generate_attachment_metadata' filter
5217+
* before preparing its response, and a callback is free to rewrite the
5218+
* attachment's post row - an optimizer that converts the file updates
5219+
* post_mime_type, for instance. The editor stores this response as its
5220+
* copy of the record rather than reading the attachment again, so
5221+
* anything stale here is what the block keeps for the session.
5222+
*
5223+
* @ticket 66056
5224+
*
5225+
* @covers WP_REST_Attachments_Controller::finalize_item
5226+
*/
5227+
public function test_finalize_response_reflects_post_row_changed_by_metadata_filter(): void {
5228+
$this->enable_client_side_media_processing();
5229+
5230+
wp_set_current_user( self::$author_id );
5231+
5232+
$request = new WP_REST_Request( 'POST', '/wp/v2/media' );
5233+
$request->set_header( 'Content-Type', 'image/jpeg' );
5234+
$request->set_header( 'Content-Disposition', 'attachment; filename=filtered-post-row.jpg' );
5235+
$request->set_param( 'generate_sub_sizes', false );
5236+
$request->set_param( 'title', 'Original title' );
5237+
$request->set_body( (string) file_get_contents( DIR_TESTDATA . '/images/canola.jpg' ) );
5238+
5239+
$attachment_id = rest_get_server()->dispatch( $request )->get_data()['id'];
5240+
5241+
$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/sideload" );
5242+
$request->set_header( 'Content-Type', 'image/jpeg' );
5243+
$request->set_header( 'Content-Disposition', 'attachment; filename=filtered-post-row-150x150.jpg' );
5244+
$request->set_param( 'image_size', 'thumbnail' );
5245+
$request->set_body( (string) file_get_contents( DIR_TESTDATA . '/images/test-image.jpg' ) );
5246+
5247+
$thumbnail_data = rest_get_server()->dispatch( $request )->get_data();
5248+
5249+
// Stands in for a plugin that rewrites the post row as the metadata is
5250+
// generated. Added after the upload so only finalize runs it.
5251+
add_filter(
5252+
'wp_generate_attachment_metadata',
5253+
static function ( $metadata, $id ) {
5254+
wp_update_post(
5255+
array(
5256+
'ID' => $id,
5257+
'post_title' => 'Rewritten while generating metadata',
5258+
)
5259+
);
5260+
return $metadata;
5261+
},
5262+
10,
5263+
2
5264+
);
5265+
5266+
$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" );
5267+
$request->set_param( 'sub_sizes', array( $thumbnail_data ) );
5268+
5269+
$response = rest_get_server()->dispatch( $request );
5270+
$this->assertSame( 200, $response->get_status() );
5271+
5272+
$this->assertSame(
5273+
'Rewritten while generating metadata',
5274+
get_post( $attachment_id )->post_title,
5275+
'The filter should have rewritten the stored post row.'
5276+
);
5277+
5278+
$this->assertSame(
5279+
'Rewritten while generating metadata',
5280+
$response->get_data()['title']['raw'],
5281+
'The finalize response should carry the rewritten post row, not the row as it was before the metadata was generated.'
5282+
);
5283+
}
5284+
5285+
/**
5286+
* Verifies that finalize fails when the attachment no longer exists by the
5287+
* time its response is prepared.
5288+
*
5289+
* A 'wp_generate_attachment_metadata' callback that rejects the file and
5290+
* deletes the attachment must not be answered with a 200 built from the row
5291+
* as it was before the callback ran: the editor would store that stale
5292+
* record and report the upload as complete.
5293+
*
5294+
* @ticket 66056
5295+
*
5296+
* @covers WP_REST_Attachments_Controller::finalize_item
5297+
*/
5298+
public function test_finalize_fails_when_metadata_filter_deletes_attachment(): void {
5299+
$this->enable_client_side_media_processing();
5300+
5301+
wp_set_current_user( self::$author_id );
5302+
5303+
$request = new WP_REST_Request( 'POST', '/wp/v2/media' );
5304+
$request->set_header( 'Content-Type', 'image/jpeg' );
5305+
$request->set_header( 'Content-Disposition', 'attachment; filename=deleted-while-finalizing.jpg' );
5306+
$request->set_param( 'generate_sub_sizes', false );
5307+
$request->set_body( (string) file_get_contents( DIR_TESTDATA . '/images/canola.jpg' ) );
5308+
5309+
$attachment_id = rest_get_server()->dispatch( $request )->get_data()['id'];
5310+
5311+
add_filter(
5312+
'wp_generate_attachment_metadata',
5313+
static function ( $metadata, $id ) {
5314+
wp_delete_attachment( $id, true );
5315+
return $metadata;
5316+
},
5317+
10,
5318+
2
5319+
);
5320+
5321+
$request = new WP_REST_Request( 'POST', "/wp/v2/media/{$attachment_id}/finalize" );
5322+
$request->set_param( 'sub_sizes', array() );
5323+
5324+
$response = rest_get_server()->dispatch( $request );
5325+
5326+
$this->assertErrorResponse( 'rest_post_invalid_id', $response, 404 );
5327+
}
5328+
51315329
/**
51325330
* Tests that the sideload route declares `convert_format` as a boolean arg.
51335331
*

0 commit comments

Comments
 (0)